PR: #108 — Batch processing for retroactively enrolling members to courses.
andrewlimaza → dev | 15 files, +450 -46 lines
#108

Summary
Solid foundation for retroactive batch enrollment via Action Scheduler — the class structure, pagination via offset chaining, and per-course processed-levels tracking are all sound. One correctness issue undermines the stated goal for old courses; a few minor things to clean up before merge. Not blocking, but the major issue should be fixed.

Issues

• Major includes/batch-enrollment.php:206-218 (get_published_course_ids_for_post_type) — The daily cron is supposed to retroactively enroll members in courses that existed before this plugin version, but the query filters AND p.post_modified >= %s (last 24 hours). This means any course that hasn't been touched in more than a day is silently skipped every single daily run, forever. The PROCESSED_LEVELS_META already prevents re-processing enrolled levels — the time filter is redundant and actively breaks the retroactive goal. Remove the post_modified filter entirely, or query for courses that don't yet have _pmpro_courses_batch_enrollment_levels set and let the metadata be the deduplication gate.

• Minor includes/modules/learndash.php:228 (retroactive_enroll_user) — ld_update_course_access() does not have a documented return value and returns void in some LearnDash versions. if ( ! $result ) will always be truthy when it returns null/false, logging a spurious error on every successful enrollment. Either drop the return-value check or verify the current LD version actually returns a boolean here.

• Minor includes/batch-enrollment.php:96-109 (schedule) vs process_batch:163-172 — The first batch task goes through PMPro_Action_Scheduler::instance()->maybe_add_task() (presumably deduplication-aware), but chained batches call as_enqueue_async_action() directly. This inconsistency is probably intentional (chained offsets shouldn't deduplicate), but a comment explaining why the two paths differ would prevent someone from "fixing" it later.

• Minor includes/batch-enrollment.php:278-280 (init guard) — The class_exists('PMPro_Action_Scheduler') check runs at include-time. Since batch-enrollment.php is require_once'd directly in pmpro-courses.php (before plugins_loaded fires), this guard could fail if PMPro loads after pmpro-courses depending on alphabetical plugin order. Wrapping the init() call in add_action('plugins_loaded', ...) with a late priority would make this robust.

Looks Good

get_active_members() — correct use of ORDER BY user_id before LIMIT/OFFSET ensures consistent pagination across batch runs. array_map('intval', $level_ids) + sort() before building placeholders prevents both injection and unstable query plans.

maybe_schedule_for_course() — the autosave/revision guards and the post_status === 'publish' check are all in the right place. Clean.

Questions

1. The PR description notes this "won't re-run batches whenever a member is added to a level programmatically and bypasses any WordPress hooks." Is that known gap documented anywhere user-facing, or does it need a notice in the admin or release notes?

2. tutorlms.php:retroactive_enroll_user — do_enroll( $user_id, 0, $course_id ) matches the pre-existing pattern at line 295, but TutorLMS's documented signature is do_enroll($course_id, $order_id, $user_id). Can you confirm this arg order is correct for the TutorLMS version being targeted? The pre-existing code may have a latent bug here that this PR is replicating.